Skip to content

fix: keep module defaults for fields a preset leaves undefined - #47

Merged
kamilmysliwiec merged 1 commit into
nestjs:masterfrom
mwawacek:scout/undefined-option-defaults-v2
Nov 9, 2026
Merged

kamilmysliwiec merged 1 commit into
nestjs:masterfrom
mwawacek:scout/undefined-option-defaults-v2

Conversation

@mwawacek

@mwawacek mwawacek commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior?

A circuit breaker, bulkhead or backoff field that a preset (or ResilienceService.create()) sets to undefined replaces the module default instead of inheriting it. ConfigService.get() gives undefined for a setting that isn't set:

ResilienceModule.forRootAsync({
  inject: [ConfigService],
  useFactory: (config: ConfigService) => ({
    defaults: { circuitBreaker: { minimumCalls: 1 }, bulkhead: { maxConcurrent: 2 } },
    presets: {
      carrier: {
        circuitBreaker: { minimumCalls: config.get<number>('CARRIER_MIN_CALLS') }, // undefined
        bulkhead: { maxConcurrent: config.get<number>('CARRIER_MAX_CONCURRENT') }, // undefined
      },
    },
  }),
});

resilience.circuitBreaker('carrier').options.minimumCalls; // 10 (built-in), expected 1
resilience.bulkhead('carrier').maxConcurrent;              // 10 (built-in), expected 2

The same happens to retry.backoff fields (backoff: { delay: undefined } drops defaults.retry.backoff.delay). Top-level retry options and decorator options already skip undefined fields (definedFields()). (A value parsed from an unset variable, such as Number(process.env.X), is NaN instead, which already fails the boot naming the option.)

Issue Number: N/A

What is the new behavior?

undefined fields are dropped before circuit breaker, bulkhead and backoff options are merged over the module defaults, as they already are for retry options. definedFields() moves to utils/options.util.ts so the registries can use it without importing PolicyFactory.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

A field set to undefined now always inherits. So bulkhead: { queueTimeout: undefined } in a preset no longer removes a defaults.bulkhead.queueTimeout; there is no other value for "no limit" there (Infinity is rejected as a duration).

Drop `undefined` fields of circuit breaker, bulkhead and backoff options
before merging them over the module defaults, as retry options already
do. A preset or `ResilienceService.create()` field set to `undefined`,
such as an unset setting from `ConfigService.get()`, replaced the module
default for that field: `circuitBreaker: { minimumCalls: undefined }`
under `defaults.circuitBreaker.minimumCalls: 1` gave the built-in 10.

Move `definedFields()` to `utils/options.util.ts`, so the registries can
use it without importing `PolicyFactory`.
@kamilmysliwiec
kamilmysliwiec merged commit 474f311 into nestjs:master Nov 9, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants